From b91c54f3247a010ade59e251334c67facc712596 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 26 Aug 2016 12:34:17 +0300 Subject: [PATCH] Refactor EncodableResolve::into_resolve --- src/cargo/core/resolver/encode.rs | 139 +++++++++++++++--------------- tests/bad-config.rs | 30 +++++++ 2 files changed, 100 insertions(+), 69 deletions(-) diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 3e049ca5c..1f79fbdc4 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, BTreeMap}; +use std::collections::{HashMap, HashSet, BTreeMap}; use std::fmt; use std::str::FromStr; @@ -24,17 +24,6 @@ impl EncodableResolve { pub fn into_resolve(self, ws: &Workspace) -> CargoResult { let path_deps = build_path_deps(ws); - let mut g = Graph::new(); - let mut tmp = HashMap::new(); - let mut replacements = HashMap::new(); - - let id2pkgid = |id: &EncodablePackageId| { - to_package_id(&id.name, &id.version, id.source.as_ref(), &path_deps) - }; - let dep2pkgid = |dep: &EncodableDependency| { - to_package_id(&dep.name, &dep.version, dep.source.as_ref(), &path_deps) - }; - let packages = { let mut packages = self.package.unwrap_or(Vec::new()); if let Some(root) = self.root { @@ -43,58 +32,83 @@ impl EncodableResolve { packages }; - let ids = try!(packages.iter().map(&dep2pkgid) - .collect::>>()); + // `PackageId`s in the lock file don't include the `source` part + // for workspace members, so we reconstruct proper ids. + let (live_pkgs, all_pkgs) = { + let mut live_pkgs = HashMap::new(); + let mut all_pkgs = HashSet::new(); + for pkg in packages.iter() { + let enc_id = EncodablePackageId { + name: pkg.name.clone(), + version: pkg.version.clone(), + source: pkg.source.clone(), + }; - { - let mut register_pkg = |pkgid: &PackageId| { - let precise = pkgid.source_id().precise() - .map(|s| s.to_string()); - if tmp.insert(pkgid.clone(), precise).is_some() { + if !all_pkgs.insert(enc_id.clone()) { return Err(internal(format!("package `{}` is specified twice in the lockfile", - pkgid.name()))); + pkg.name))); } - g.add(pkgid.clone(), &[]); - Ok(()) - }; + let id = match pkg.source.as_ref().or(path_deps.get(&pkg.name)) { + // We failed to find a local package in the workspace. + // It must have been removed and should be ignored. + None => continue, + Some(source) => try!(PackageId::new(&pkg.name, &pkg.version, source)) + }; - for id in ids.iter() { - try!(register_pkg(id)); + assert!(live_pkgs.insert(enc_id, (id, pkg)).is_none()) } - } + (live_pkgs, all_pkgs) + }; - { - let mut add_dependencies = |id: &PackageId, pkg: &EncodableDependency| - -> CargoResult<()> { - if let Some(ref replace) = pkg.replace { - let replace = try!(id2pkgid(replace)); - let replace_precise = tmp.get(&replace).map(|p| { - replace.with_precise(p.clone()) - }).unwrap_or(replace); - replacements.insert(id.clone(), replace_precise); - assert!(pkg.dependencies.is_none()); - return Ok(()) + let lookup_id = |enc_id: &EncodablePackageId| -> CargoResult> { + match live_pkgs.get(enc_id) { + Some(&(ref id, _)) => Ok(Some(id.clone())), + None => if all_pkgs.contains(enc_id) { + // Package is found in the lockfile, but it is + // no longer a member of the workspace. + Ok(None) + } else { + Err(internal(format!("package `{}` is specified as a dependency, \ + but is missing from the package list", enc_id))) } + } + }; + let g = { + let mut g = Graph::new(); + + for &(ref id, _) in live_pkgs.values() { + g.add(id.clone(), &[]); + } + + for &(ref id, ref pkg) in live_pkgs.values() { let deps = match pkg.dependencies { Some(ref deps) => deps, - None => return Ok(()), + None => continue }; + for edge in deps.iter() { - let to_depend_on = try!(id2pkgid(edge)); - let precise_pkgid = - tmp.get(&to_depend_on) - .map(|p| to_depend_on.with_precise(p.clone())) - .unwrap_or(to_depend_on.clone()); - g.link(id.clone(), precise_pkgid); + if let Some(to_depend_on) = try!(lookup_id(edge)) { + g.link(id.clone(), to_depend_on); + } } - Ok(()) - }; + } + g + }; - for (id, ref pkg) in ids.iter().zip(packages) { - try!(add_dependencies(id, pkg)); + let replacements = { + let mut replacements = HashMap::new(); + for &(ref id, ref pkg) in live_pkgs.values() { + if let Some(ref replace) = pkg.replace { + assert!(pkg.dependencies.is_none()); + if let Some(replace_id) = try!(lookup_id(replace)) { + replacements.insert(id.clone(), replace_id); + } + } } - } + replacements + }; + let mut metadata = self.metadata.unwrap_or(BTreeMap::new()); // Parse out all package checksums. After we do this we can be in a few @@ -118,13 +132,14 @@ impl EncodableResolve { for (k, v) in metadata.iter().filter(|p| p.0.starts_with(prefix)) { to_remove.push(k.to_string()); let k = &k[prefix.len()..]; - let id: EncodablePackageId = try!(k.parse().chain_error(|| { + let enc_id: EncodablePackageId = try!(k.parse().chain_error(|| { internal("invalid encoding of checksum in lockfile") })); - let id = try!(to_package_id(&id.name, - &id.version, - id.source.as_ref(), - &path_deps)); + let id = match lookup_id(&enc_id) { + Ok(Some(id)) => id, + _ => continue, + }; + let v = if v == "" { None } else { @@ -187,20 +202,6 @@ fn build_path_deps(ws: &Workspace) -> HashMap { } } -fn to_package_id(name: &str, - version: &str, - source: Option<&SourceId>, - path_sources: &HashMap) - -> CargoResult { - if let Some(source) = source.or(path_sources.get(name)) { - PackageId::new(name, version, source) - } else { - let dummy_source = SourceId::from_url("path+file:///dummy_path").unwrap(); - PackageId::new(name, version, &dummy_source) - } -} - - #[derive(RustcEncodable, RustcDecodable, Debug, PartialOrd, Ord, PartialEq, Eq)] pub struct EncodableDependency { name: String, @@ -210,7 +211,7 @@ pub struct EncodableDependency { replace: Option, } -#[derive(Debug, PartialOrd, Ord, PartialEq, Eq)] +#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, Hash, Clone)] pub struct EncodablePackageId { name: String, version: String, diff --git a/tests/bad-config.rs b/tests/bad-config.rs index b62939c84..f4a28e51d 100644 --- a/tests/bad-config.rs +++ b/tests/bad-config.rs @@ -319,6 +319,36 @@ Caused by: ")); } +#[test] +fn bad_dependency_in_lockfile() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + "#) + .file("src/lib.rs", "") + .file("Cargo.lock", r#" + [root] + name = "foo" + version = "0.0.1" + dependencies = [ + "bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + ] + "#); + p.build(); + + assert_that(p.cargo("build").arg("--verbose"), + execs().with_status(101).with_stderr("\ +[ERROR] failed to parse lock file at: [..] + +Caused by: + package `bar 0.1.0 ([..])` is specified as a dependency, but is missing from the package list +")); + +} + #[test] fn bad_git_dependency() { let foo = project("foo") -- 2.30.2